feat(delegation): per-subagent terminal sandbox isolation for delegate_task - #83430

AI code review — automated review for reference, author can ignore or act on any point.

feat(delegation): per-subagent terminal sandbox isolation for delegate_task — no blocking issues found. A few minor observations:

1. tools/delegate_tool.py::_SANDBOXABLE_BACKENDS includes singularity, and _register_child_sandbox_overrides registers {env_type}_image → singularity_image. Confirm tools/terminal_tool.py actually honors a per-task singularity_image override; if it doesn't, sandbox=True on singularity would register an ineffective key — exactly the false-isolation failure this PR exists to prevent.

2. Container lifecycle: _clear_child_sandbox only drops registry overrides; container removal depends on the child's close() → cleanup_vm path. A child killed by a hard interrupt, a parent crash, or a process kill leaves an orphan container with no reaper. Consider a registry stamp + cleanup sweep on next gateway start so stale sandbox containers don't accumulate.

3. In the sandbox path, _seed_child_terminal_env records the parent's cwd for the child, but the child's fresh container starts at its image's default working directory — so the recorded cwd may not match the container's actual cwd. Minor inconsistency, same as the shared path, but worth a comment so future readers don't assume the record equals the container state.

4. sandbox normalization happens at two sites (delegate_task top-level and _run_single_child per-task via is_truthy_value), and the schema exposes it in both the top-level and per-task positions — the double surface is fine, but the two normalization sites could drift; a single normalization helper would keep the semantics in one place.

@Enough1122 — addressed all four points. Also rebased onto current main (it had moved ~15 commits since the PR branched on Aug 10; the PR is now mergeable). All citations below are at the rebased head 94e0c92a64.

1. Singularity override — confirmed honored, and now pinned by a test. Both container-creation sites resolve overrides.get("singularity_image") or config["singularity_image"] for env_type == "singularity" (tools/terminal_tool.py:2087 in ensure_task_env, :2612 in the env build), and singularity_image is in _ISOLATION_OVERRIDE_KEYS (:1336), so _resolve_container_task_id (:1354) treats it as a real isolation signal — not a dead registration. The probe found a genuine coverage gap: the suite had docker/modal/daytona registration tests but no singularity mirror. Added test_singularity_registers_singularity_image, and mutation-checked it: removing singularity from _SANDBOXABLE_BACKENDS (tools/delegate_tool.py:67) fails exactly that test (ValueError at the override-registration site), restored → passes.

2. Orphan containers — the existing reapers already cover the sandboxed child (all three predate this PR, which touches none of them):

• Idle reaper _cleanup_inactive_envs (terminal_tool.py:1948, 300s default, worker at :2015) sweeps _active_environments by _last_activity and calls env.cleanup(). The child's container is keyed under its own task_id with an activity stamp at creation (:2080/:2130/:2765) refreshed on every use — so if the child is hard-interrupted while the parent process stays alive, its container goes idle and is reclaimed within the lifetime window. The normal path removes it eagerly: child.close() → cleanup_vm, then the override is cleared (delegate_tool.py:3305-3309).
• _atexit_cleanup (terminal_tool.py:2259) closes all remaining envs on graceful exit.
• If the parent process itself dies (crash/kill), the Docker orphan reaper _maybe_reap_docker_orphans (:1106, issue Bug: Docker containers accumulate - /stop doesn't clean, multiple per session, async race #20561, once per process) reclaims stale hermes-tagged containers at next startup — which is also the documented reclaim path for persist-mode idle envs (:2210). The only residue after a hard interrupt is the in-memory registry override entry, which dies with the process and cannot accumulate across restarts.

3. cwd record vs container cwd — they can't diverge at creation: the record is the source of the workdir. The env build derives the container's working directory from the child's session-cwd record (cwd = overrides.get("cwd") or get_session_cwd(task_id) or config["cwd"], terminal_tool.py:2620, passed to docker run -w per the guard at :2638), and the record was seeded from the parent at spawn (delegate_tool.py:2421). Post-creation cds update the record in lockstep. The one divergence edge — a host path remapped to /workspace by _is_unusable_container_cwd (:1497, applied at :2638) — is pre-existing, logged, and identical on the shared path. Added a docstring sentence in _seed_child_terminal_env (delegate_tool.py:2410) making the record→workdir flow explicit so future readers don't assume otherwise.

4. Normalization drift — agreed, folded into one helper. There were three is_truthy_value sites (top-level delegate_tool.py:3626, batch validation :3727, per-task :3886/:3912); they now share _normalize_sandbox(value, default) (:2310), and the redundant if sandbox is not None guard at the top-level site is gone (is_truthy_value(None, default=False) already returns False). Semantics unchanged.

Verification: pytest tests/tools/test_delegate_sandbox_isolation.py — 22 passed (21 existing + the new singularity test); 166 passed across the delegate + docker session-isolation suites on the rebased head.